Allow callers to ignore selected critical extensions#485
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #485 +/- ##
==========================================
+ Coverage 97.01% 97.05% +0.04%
==========================================
Files 20 20
Lines 3950 4005 +55
==========================================
+ Hits 3832 3887 +55
Misses 118 118 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1667c76 to
88fc6de
Compare
An earlier discussion on this: #232 (comment) |
|
Thanks for linking this, I have not seen this. I think my approach matches the spirit of this comment moderately well:
In my approach, the user still needs to explicitly declare which critical extensions are to be ignored, and is free to do any processing before or after. The user will need to reparse the certificate to extract the extensions, but I think this is unavoidable in any case: rustls-webpki cannot offer much help in parsing the extensions it does not recognize. At best, I guess what rustls-webpki could offer is an additional method on that just iterates over the DER bytes of extensions. We could actually go one step further, and introduce a public API for extension that just matches the RFC 5280 definition: and have the |
Add ExtensionId and an opt-in EndEntityCert constructor that accepts a DER OID allowlist for unsupported critical extensions on the leaf certificate. Existing TryFrom parsing remains strict by default. Add PathBuilder::with_ignored_critical_extensions so callers can explicitly apply the same allowlist when path building parses intermediate certificates. PathBuilder remains strict for intermediates unless this builder option is used. Supported extensions are still parsed and validated normally; the allowlist only suppresses UnsupportedCriticalExtension for exact unsupported OID matches. Add integration tests for leaf and intermediate allowlisting, exact-match behavior, and supported-extension parsing.
88fc6de to
d257492
Compare
|
Hey, I rebased the branch to use the new |
|
An X.509 certificate with critical extensions that uses "private critical extensions" is not a WebPKI certificate so it is out of scope of this library. If there is interest in supporting non-WebPKI-conformant certificates then I think it would be good if the API for that made a clear distinction between WebPKI certificate processing and non-WebPKI certificate processing. There are definitely a lot of certificate hierarchies that aren't too different from the WebPKI where this library almost works for them, so it is tempting to add features to support this. |
|
The idea I've been kicking around is to have a trait |
|
Even still, I think it would be good to have more of a distinction in the API between "Hey, I'm using this to validate a WebPKI certificate" vs. other certificates. The recent addition of things like the RequiredXIfYPresent EKU policy stuff already have started to make it difficult to see what parts of this code are relevant to the WebPKi and which aren't. With respect to this potential API, I think we should also enforce that the ignored critical extensions are NOT standardized extensions. e.g. don't let this API be used to ignore policy-related extensions or other WebPKI-relevant extensions. Otherwise, we encourage people to build applications that are incompatible with more fully-featured standard WebPKI-compatible libraries. One potential way to do this would be to validate that the extensions to ignore have OIDs that aren't in standardized OID namespaces. The main footgun in APIs like this, which let the application process extensions themselves, is that now that extension processing doesn't influence path building. This is most likely to happen when the extensions are in CA certificates. I suggest that any such API accept separate sets of extensions for end-entity and CA certificates. In the case of the OP's issue, based on the original summary, it seems likely that those extensions are in end-entity certificates? |
|
I think people use There would be value in implementing path building API which mirrors closely the algorithm described in RFC 5280, including the specified inputs, and hooks into each step. Many extensions describe their processing in terms of how to modify the RFC 5280 algorithm. See, for example, RFC 5937, Using Trust Anchor Constraints during Certification Path Processing: it tells you how to modify the inputs to the RFC 5280 algorithm, but then doesn't require any special processing during building, because the modification to the inputs will affect how the standard algorithm operates. Doing this is impossible with current
My change does not disable processing of the critical extensions that are part of WebPKI, and in fact I added a test for it: ignored_critical_extensions_do_not_disable_supported_extension_parsing
My change does exactly that, compare ignored_critical_extension_on_end_entity vs. ignored_critical_extension_on_end_entity, although I did not exactly intend it to be that way, it just ended up being forced by the existing API, which required parsing the
I actually struggle with two distinct kinds of private extensions, believe it or not. Once is on end-entity only, and the other affects the entire path. I will give you an example of a public extensions, however. Consider the processing procedure described in RFC 5913. It plays absolutely no role during actual path building (at least so far as I understand it), and can be applied separately to the finished verified path. |
I'm working with a system that uses private critical extensions on X.509 certificate. They don't play a role during building a path, but are then used to enforce constraints on the objects being authenticated. Therefore, I'd like to be able to make
rustls-webpkiignore these extensions when building a path, and verify these later separately.I don't like how I had to add a new method to make sure that API is backwards-compatible. I think that
verify_for_usagealready has too many arguments as it is. I'd suggest adding a new API that's extensible by design: it should take anOptionssort of an object as argument, the fields of which should be private; this way new functionality can be added by adding new public methods on theOptionsobject.I could do it in a separate PR, and then once it's merged, rebase this one on top of the other one. Thoughts?